Skip to content

Add analyzer section in project.assets.json file - #7464

Merged
martinrrm merged 15 commits into
devfrom
dev-mruizmares-analyzer-assets
Jul 17, 2026
Merged

Add analyzer section in project.assets.json file#7464
martinrrm merged 15 commits into
devfrom
dev-mruizmares-analyzer-assets

Conversation

@martinrrm

@martinrrm martinrrm commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Bug

Progress: NuGet/Home#6279
SDK PR (draft): dotnet/sdk#54646

Description

Today project.assets.json does not record which analyzers a package contributes, and analyzers are applied regardless of PrivateAssets / ExcludeAssets / IncludeAssets. This PR implements the NuGet restore half of the "indicate analyzer assets in project.assets.json" feature (spec: NuGet/Home#14455). The SDK consumption half ships as a separate dotnet/sdk PR.

When a project opts in with <RestoreEnableAnalyzerAssets>true</RestoreEnableAnalyzerAssets>, restore now:

  • Emits an analyzers group under each package in the assets file, listing every analyzer assembly. Detection uses a new AnalyzerAssemblies pattern in ManagedCodeConventions (any .dll under analyzers/ at any depth, excluding satellite .resources.dll), so the layout isn't assumed to be fixed and analyzer discovery is shared with the rest of the content model.
  • Respects asset filtering. PrivateAssets, ExcludeAssets, and IncludeAssets filter analyzers like any other asset type. Analyzers excluded or transitively suppressed are written as a _._ placeholder (consistent with compile / runtime / native).
  • Attaches selection metadata to each entry: codeLanguage (cs/vb/fs/any) and, when present in the path, compilerApiVersion (roslynX.Y) — mirroring how content files carry codeLanguage — so the SDK selects applicable analyzers from metadata instead of parsing paths. The metadata is derived by scanning the path segments, since real packages place the language at a variable depth (for example analyzers/dotnet/roslynX.Y/cs/).

Gating. The feature is opt-in and only honored for projects targeting .NET 11 or greater. The .NET 11+ gate is applied at evaluation time in NuGet.targets (mirroring NuGetAuditMode), and the resulting value is honored per target framework: it flows through TargetFrameworkInformation.RestoreEnableAnalyzerAssets and is read in code by every restore entry point — CLI/DG-spec (MSBuildRestoreUtility), VS nomination (PackageSpecFactory), and static-graph restore (MSBuildStaticGraphRestore, which previously did not honor the property at all). A multi-targeted net10.0;net11.0 project therefore emits the analyzers group only for net11.0.

Telemetry. ProjectRestoreInformation reports AnalyzerAssets.Enabled (opt-in state) and AnalyzerAssets.Count (number of analyzer assets emitted, excluding _._ placeholders). Richer adoption/impact telemetry to inform the default-on rollout ships as a follow-up.

Public API & serialization. Adds TargetFrameworkInformation.RestoreEnableAnalyzerAssets, LockFileTargetLibrary.AnalyzerAssets, LockFileItem.CompilerApiVersionProperty, and the ManagedCodeConventions.ManagedCodePatterns.AnalyzerAssemblies pattern (with the AnalyzerAssembly property name). The Newtonsoft.Json and System.Text.Json paths round-trip the new analyzers group and its metadata; the per-framework restore property round-trips through PackageSpecWriter and the streaming reader. The lock-file cache key includes the per-framework flag so toggling it invalidates correctly.

PR Checklist

  • Meaningful title, helpful description and a linked NuGet/Home issue
  • Added tests
  • Link to an issue or pull request to update docs if this PR changes settings, environment variables, new feature, etc.

@martinrrm
martinrrm requested a review from a team as a code owner June 8, 2026 23:06
@martinrrm
martinrrm requested review from kartheekp-ms and zivkan June 8, 2026 23:06
jebriede
jebriede previously approved these changes Jun 9, 2026

@jebriede jebriede left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving with a few suggestions.

Comment thread src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs Outdated
Comment thread src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/LockFileUtils.cs Outdated
Comment thread src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/LockFileUtils.cs Outdated
Comment thread src/NuGet.Core/NuGet.ProjectModel/LockFile/LockFileFormat.cs
jebriede
jebriede previously approved these changes Jun 15, 2026

@jebriede jebriede left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving with an optimization suggestion.

Comment thread src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs Outdated

@nkolev92 nkolev92 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're introducing a new implementation pattern in a couple of different places, but we already have an established one.

Comment thread src/NuGet.Core/NuGet.Build.Tasks/NuGet.targets Outdated
Comment thread src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/LockFileUtils.cs Outdated
@dotnet-policy-service dotnet-policy-service Bot added the Status:No recent activity PRs that have not had any recent activity and will be closed if the label is not removed label Jun 25, 2026
@martinrrm martinrrm removed the Status:No recent activity PRs that have not had any recent activity and will be closed if the label is not removed label Jun 25, 2026
@martinrrm
martinrrm force-pushed the dev-mruizmares-analyzer-assets branch 2 times, most recently from 963cbee to b7099ed Compare June 26, 2026 00:42
Comment thread src/NuGet.Core/NuGet.Commands/RestoreCommand/LockFileBuilder.cs Outdated
Comment thread src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs Outdated
Comment thread src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs
Comment thread src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs Outdated
Comment thread src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs Outdated
Comment thread src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs
martinrrm added a commit that referenced this pull request Jun 30, 2026
Address PR feedback (#7464): use the NoAllocEnumerate helper for the IList
iterations in PopulateAnalyzerAssetsTelemetry.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
martinrrm added a commit that referenced this pull request Jun 30, 2026
Address PR feedback (#7464): look up TargetFrameworkInformation by (framework, alias)
via GetNearestTargetFramework instead of framework-only, in LockFileBuilder and the
analyzer-assets telemetry, so multi-targeted projects with aliased frameworks resolve
to the correct target framework.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
martinrrm added a commit that referenced this pull request Jun 30, 2026
Address PR feedback (#7464): treat RestoreEnableAnalyzerAssets as a project-wide
opt-in instead of per target framework. When any target framework opts in, analyzer
assets are honored for every target framework, mirroring how package pruning is
enabled project-wide once any framework qualifies (PackageSpecFactory.GetPackagePruningDefault).
The per-framework opt-in is already gated to .NET 11+ in NuGet.targets, so only a
qualifying framework can enable it.

Add a multi-targeted restore test asserting a non-opted-in framework still honors
analyzers when another framework opts in.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@martinrrm
martinrrm requested a review from nkolev92 July 2, 2026 20:36
martinrrm added a commit that referenced this pull request Jul 2, 2026
Address PR feedback (#7464): analyzer assets restore was force-disabled below
.NET 11 even when explicitly opted in. Mirror package pruning instead — default
to off, but honor an explicit RestoreEnableAnalyzerAssets opt-in (or opt-out) on
any target framework, so users can opt into the new behavior before it is enabled
by default. The net11 version gate only ever forced off the explicit opt-in; a
future default-on can gate itself the way RestorePackagePruningDefault does.

Update the availability tests: the opt-in is now honored on all frameworks, and
the value defaults to false when unset.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment thread src/NuGet.Core/NuGet.ProjectModel/TargetFrameworkInformation.cs Outdated
Comment thread src/NuGet.Core/NuGet.Build.Tasks.Console/MSBuildStaticGraphRestore.cs Outdated
Comment thread src/NuGet.Core/NuGet.Commands/RestoreCommand/Utility/LockFileUtils.cs Outdated
@martinrrm
martinrrm requested a review from nkolev92 July 7, 2026 20:00
Comment thread src/NuGet.Core/NuGet.Commands/RestoreCommand/RestoreCommand.cs
@martinrrm
martinrrm requested a review from Nigusu-Allehu July 16, 2026 23:13
nkolev92
nkolev92 previously approved these changes Jul 16, 2026
martinrrm and others added 15 commits July 17, 2026 10:00
…ventions detection

Move the RestoreEnableAnalyzerAssets opt-in gating into code, per target
framework. The value now flows through TargetFrameworkInformation and is read
by every restore entry point, including static-graph restore which previously
ignored the property entirely. Removes the project-level
ProjectRestoreMetadata.RestoreEnableAnalyzerAssets and the NuGet.targets OR.

Detect analyzers via a new AnalyzerAssemblies pattern in ManagedCodeConventions
instead of the hand-rolled IsAnalyzerAssetPath, so detection is shared with the
content model and matches the analyzers folder case-insensitively.

Clarify the analyzer metadata derivation comment and trim an allocation in the
compiler-version normalization.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add five properties to the ProjectRestoreInformation event to size the impact
of enabling RestoreEnableAnalyzerAssets by default before the rollout:

- AnalyzerAssets.Excluded.Count
- AnalyzerAssets.PackagesWithAnalyzers.Count
- AnalyzerAssets.PackagesWithExcludedAnalyzers.Count
- AnalyzerAssets.ExcludedByPrivateAssets.Count
- AnalyzerAssets.ExcludedByExcludeAssets.Count

The counts are derived from the resolved dependency graphs and the package
file lists, so they are reported on every full restore regardless of whether
the feature is currently enabled. This lets us measure how many packages and
analyzer assemblies would stop being applied once PrivateAssets/ExcludeAssets
are honored. AnalyzerAssets.Count is redefined as the number of analyzer
assemblies that would apply (computed the same way), replacing the previous
count of analyzer assets written to the assets file.

Analyzer detection mirrors ManagedCodeConventions.ManagedCodePatterns.
AnalyzerAssemblies as a cheap string check, avoiding a content-item collection
allocation per package on the restore path.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address spec review feedback (Client.Engineering#3855): the rollout decision is
driven by whether a package's analyzers are affected, not by how many individual
analyzer assemblies it ships.

- Remove AnalyzerAssets.Count (per-assembly applied count).
- Replace AnalyzerAssets.Excluded.Count with a binary AnalyzerAssets.Excluded
  (true when any package's analyzers would be filtered out for the project).

The package-level counts (PackagesWithAnalyzers.Count, PackagesWithExcludedAnalyzers.Count,
ExcludedByPrivateAssets.Count, ExcludedByExcludeAssets.Count) are unchanged. Per-package
detection now uses a HashSet rather than per-assembly counts.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address PR feedback (#7464): use the NoAllocEnumerate helper for the IList
iterations in PopulateAnalyzerAssetsTelemetry.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address PR feedback (#7464): look up TargetFrameworkInformation by (framework, alias)
via GetNearestTargetFramework instead of framework-only, in LockFileBuilder and the
analyzer-assets telemetry, so multi-targeted projects with aliased frameworks resolve
to the correct target framework.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address PR feedback (#7464): treat RestoreEnableAnalyzerAssets as a project-wide
opt-in instead of per target framework. When any target framework opts in, analyzer
assets are honored for every target framework, mirroring how package pruning is
enabled project-wide once any framework qualifies (PackageSpecFactory.GetPackagePruningDefault).
The per-framework opt-in is already gated to .NET 11+ in NuGet.targets, so only a
qualifying framework can enable it.

Add a multi-targeted restore test asserting a non-opted-in framework still honors
analyzers when another framework opts in.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address PR feedback (#7464): analyzer assets restore was force-disabled below
.NET 11 even when explicitly opted in. Mirror package pruning instead — default
to off, but honor an explicit RestoreEnableAnalyzerAssets opt-in (or opt-out) on
any target framework, so users can opt into the new behavior before it is enabled
by default. The net11 version gate only ever forced off the explicit opt-in; a
future default-on can gate itself the way RestorePackagePruningDefault does.

Update the availability tests: the opt-in is now honored on all frameworks, and
the value defaults to false when unset.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
RestoreEnableAnalyzerAssets was stored per target framework on
TargetFrameworkInformation, but analyzer assets are a project-wide opt-in.
Move it to ProjectRestoreMetadata (mirroring UsingMicrosoftNETSdk /
RestoreUseLegacyDependencyResolver / RestoreDoNotWriteDependencyGraphSpec) so
consumers read a single project-level value instead of scanning every
target framework.

- Model: add the property (plus Equals/GetHashCode/Clone) to
  ProjectRestoreMetadata and remove it from TargetFrameworkInformation.
- Serialization: read and write it under the "restore" metadata section.
- Readers: PackageSpecFactory, MSBuildStaticGraphRestore and
  MSBuildRestoreUtility now set it at the project level.
- LockFileBuilder and restore telemetry read the project-level value directly.
- NuGet.targets: emit RestoreEnableAnalyzerAssets on the ProjectSpec restore
  graph entry (not the per-framework entry) so the task-based restore path
  captures the opt-in; add a GenerateRestoreGraphFile functional test covering
  both the standard and static-graph evaluation paths.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Resolve target framework information by alias instead of selecting a nearest
framework. Keep warning and dependency alias handling safe when no exact
framework metadata exists.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c1c3025b-91e0-4a52-9c20-8162710176f6
Exercise LockFileBuilder with a target graph alias that is absent from the
PackageSpec so exact alias lookup remains null-safe.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c1c3025b-91e0-4a52-9c20-8162710176f6
Preserve project-wide analyzer behavior after moving the setting to restore
metadata. Promote an enabled outer or inner build consistently across standard,
PackageSpecFactory, and static graph restore paths.

Add focused unit coverage and an SDK-next NET10/NET11 restore matrix that verifies
analyzers are written for every target framework.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c1c3025b-91e0-4a52-9c20-8162710176f6
Run the multi-target analyzer assets test on the default SDK 11 test target and align its name with the project-wide opt-in scenario.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: dc2642ca-597e-4ac9-b965-80b0fbd3b338
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants